BH Config & Carousel -> PROD-2321 #123
Conversation
There was a problem hiding this comment.
@mmattlin this looks great!!
The store is the code that actually talks to the data store, which in most cases is the API.
The factory takes the raw data from the store and transforms it into the model that the UI needs.
In your PR, the configs are really a data store. In this case, they are static config files, but I could see a future scenario where these come from an API instead.
So I think the configs should be in the work-store directory and should be accessed through the work.store.
And this means that the WorkPrices configs that I created should also be moved to the store.
…ork-factory #time 30m
| @@ -258,10 +258,6 @@ function findPhase(challenge: Challenge, phases: Array<string>): ChallengePhase | |||
| // tslint:disable-next-line: cyclomatic-complexity | |||
There was a problem hiding this comment.
we can remove this tslint exclusion
| @@ -258,10 +258,6 @@ function findPhase(challenge: Challenge, phases: Array<string>): ChallengePhase | |||
| // tslint:disable-next-line: cyclomatic-complexity | |||
| function getCost(challenge: Challenge, type: WorkType): number | undefined { | |||
There was a problem hiding this comment.
Factories should be really dumb--they just take data in one form and transform it to another.
Now that we're thinking of the configs as data, it means this getCost method is actually retrieving data directly from the store in the form of the price config, which is an anti-pattern.
Let's have the work-provider pass the WorkPrices config into the factory create method instead.
There was a problem hiding this comment.
@brooketopcoder Now I'm thinking of just returning the prices as part of the WorkTypeConfigs instead of a separate object and using that in the work.factory. Let me know if you see any issues with that.
There was a problem hiding this comment.
Disregard that last comment. We can't do that unless we import the config for the other intakes as well. Maybe a future update
…k.factory, now it gets passed in to create from work.provider #time 30m
mmattlin
changed the title
What's in this PR?
Screenshots